Fix z-boundary backward FD stencil using fields(2) instead of fields(3) by sbryngelson · Pull Request #1173 · MFlowCode/MFC

sbryngelson · 2026-02-21T03:22:35Z

Summary

Severity: CRITICAL — produces wrong results in all 3D simulations using this routine.

File: src/common/m_finite_differences.fpp, line 53

The z-direction upper-boundary backward finite difference stencil at iz_s%end uses fields(2) (the y-component) instead of fields(3) (the z-component) in the third term of the second-order backward difference.

Before

divergence = divergence + (+3._wp*fields(3)%sf(x,y,z) &
    - 4._wp*fields(3)%sf(x,y,z-1) &
    + fields(2)%sf(x,y,z-2)) / (z_cc(z) - z_cc(z-2))
!   ^^^^^^^^^ should be fields(3)

After

divergence = divergence + (+3._wp*fields(3)%sf(x,y,z) &
    - 4._wp*fields(3)%sf(x,y,z-1) &
    + fields(3)%sf(x,y,z-2)) / (z_cc(z) - z_cc(z-2))

Why this went undetected

This is a copy-paste error in the x→y→z stencil chain where only the third term of the z-direction backward stencil is wrong — the first two terms correctly use fields(3). The bug only activates at the exact upper z-boundary cells (iz_s%end) where the one-sided backward stencil is used; the interior central-difference stencil and the lower-boundary forward stencil are both correct. This makes the error localized to a thin boundary layer that is easily masked by the correct values everywhere else.

Test plan

Run 3D test case with non-trivial z-boundary divergence
Verify divergence values at iz_s%end match the y-boundary analog

🤖 Generated with Claude Code

Fixes #1194

codeant-ai · 2026-02-21T03:22:39Z

CodeAnt AI is reviewing your PR.

Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

coderabbitai · 2026-02-21T03:23:03Z

📝 Walkthrough

Walkthrough

The s_compute_fd_divergence routine in the finite differences module was corrected to reference the proper field when computing the z-direction boundary divergence contribution. The affected term now accesses the third field instead of the second field.

Changes

Cohort / File(s)	Summary
Finite Differences Boundary Fix `src/common/m_finite_differences.fpp`	Corrected field index reference in z-direction boundary divergence computation from `fields(2)` to `fields(3)` for improved accuracy in boundary derivative calculations.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Suggested labels

Review effort 2/5, size:XS

Poem

🐰 A whisker-twitch away from error
Field three now leads where two did stray
Boundaries fixed with finite precision
Divergence flows the proper way
Mathematics hops back on track!

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the bug fix: correcting a field index in the z-boundary finite difference stencil from fields(2) to fields(3).
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check	✅ Passed	The PR description is comprehensive and well-structured, addressing severity, motivation, before/after code comparison, root cause analysis, and test plan.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings (stacked PR)
📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

codeant-ai · 2026-02-21T03:24:33Z

Nitpicks 🔍

🔒 No security issues identified
⚡ Recommended areas for review Boundary-index safety The backward stencil at the upper z-boundary accesses z-1 and z-2. Ensure these indices are always valid for every call (ghost/halo cells or domain sizing). If the field arrays or z_cc do not include sufficient padding, this will read out-of-bounds or fetch invalid data. Consistency check Confirm there are no other places in the codebase that still use the y-component (`fields(2)`) for z-derivative boundary terms. The PR fixes one instance; any remaining occurrences will continue to corrupt divergence at the z-boundary. Performance / memory access The stencil reads multiple nearby entries from `fields(3)%sf` and `z_cc` repeatedly inside a tight GPU-parallel loop. This can harm memory coalescing and increase global memory traffic; caching neighbors into temporaries could reduce loads and improve throughput.

codeant-ai · 2026-02-21T03:24:36Z

CodeAnt AI finished reviewing your PR.

Copilot

Pull request overview

Fixes an incorrect component reference in the z-direction upper-boundary backward finite-difference stencil inside the shared finite-differences module, preventing corrupted divergence results at the z-boundary for 3D runs.

Changes:

Correct z-upper boundary backward-difference term to use fields(3) (z-component) rather than fields(2) (y-component).

The z-direction upper-boundary backward difference at iz_s%end uses fields(2) (y-component) instead of fields(3) (z-component) in the third term, corrupting the divergence in all 3D simulations using this finite difference routine. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

codecov · 2026-02-21T07:26:43Z

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 44.05%. Comparing base (84c46e0) to head (dc4ebd7).
⚠️ Report is 4 commits behind head on master.

Files with missing lines	Patch %	Lines
src/common/m_finite_differences.fpp	0.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1173   +/-   ##
=======================================
  Coverage   44.05%   44.05%           
=======================================
  Files          70       70           
  Lines       20498    20498           
  Branches     1990     1990           
=======================================
  Hits         9030     9030           
  Misses      10329    10329           
  Partials     1139     1139

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

sbryngelson · 2026-02-22T21:02:42Z

Superseded by #1242 (batched HPC-sensitive fixes)

Copilot AI review requested due to automatic review settings February 21, 2026 03:22

Copilot started reviewing on behalf of sbryngelson February 21, 2026 03:23 View session

codeant-ai Bot added the size:XS label Feb 21, 2026

Copilot AI reviewed Feb 21, 2026

View reviewed changes

Comment thread src/common/m_finite_differences.fpp

This comment was marked as off-topic.

Sign in to view

sbryngelson force-pushed the fix/fd-z-boundary-stencil branch from e583f31 to dc4ebd7 Compare February 21, 2026 03:53

sbryngelson mentioned this pull request Feb 21, 2026

z-boundary backward FD stencil uses fields(2) instead of fields(3) #1194

Closed

sbryngelson mentioned this pull request Feb 22, 2026

Fix 8 HPC-sensitive bugs: GPU kernels, MPI broadcast, domain decomposition #1242

Merged

5 tasks

sbryngelson closed this Feb 22, 2026

sbryngelson deleted the fix/fd-z-boundary-stencil branch February 22, 2026 21:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix z-boundary backward FD stencil using fields(2) instead of fields(3)#1173

Fix z-boundary backward FD stencil using fields(2) instead of fields(3)#1173
sbryngelson wants to merge 1 commit into
MFlowCode:masterfrom
sbryngelson:fix/fd-z-boundary-stencil

sbryngelson commented Feb 21, 2026 •

edited

Loading

Uh oh!

codeant-ai Bot commented Feb 21, 2026

Uh oh!

coderabbitai Bot commented Feb 21, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Suggested labels

Poem

Uh oh!

codeant-ai Bot commented Feb 21, 2026

Uh oh!

codeant-ai Bot commented Feb 21, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

This comment was marked as off-topic.

Uh oh!

codecov Bot commented Feb 21, 2026 •

edited

Loading

Uh oh!

sbryngelson commented Feb 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

sbryngelson commented Feb 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Before

After

Why this went undetected

Test plan

Uh oh!

codeant-ai Bot commented Feb 21, 2026

Thanks for using CodeAnt! 🎉

Uh oh!

coderabbitai Bot commented Feb 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Suggested labels

Poem

Uh oh!

codeant-ai Bot commented Feb 21, 2026

Nitpicks 🔍

Uh oh!

codeant-ai Bot commented Feb 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

This comment was marked as off-topic.

Uh oh!

codecov Bot commented Feb 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sbryngelson commented Feb 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

sbryngelson commented Feb 21, 2026 •

edited

Loading

coderabbitai Bot commented Feb 21, 2026 •

edited

Loading

codecov Bot commented Feb 21, 2026 •

edited

Loading